-
Notifications
You must be signed in to change notification settings - Fork 303
Add CSV mapper for REST server FIX #606 #607
base: develop
Are you sure you want to change the base?
Conversation
Remove sample ThreadLocals
Module CSV
Ignore not implemented test
Multiple DateFormats
12d830b
to
e226c5a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems that this PR is still work in progress. Please also avoid French comments and strictly use English. Also do not include TODOs in PRs and only use them locally to track your work before pushing or completing the PR.
Thanks in advance for your work. Please let us know when your work is ready for deeper looks.
@@ -174,6 +179,11 @@ | |||
<groupId>com.fasterxml.jackson.jaxrs</groupId> | |||
<artifactId>jackson-jaxrs-json-provider</artifactId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This dependency seems redundant as it is transitively added by oasp4j-csv
.
* @author MLAVIGNE | ||
* | ||
*/ | ||
public class CsvProviderTest { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fine if you are considering it as a TODO where you are working on. In such case please comment your PR and let us know when the time has come to review the PR.
Otherwise if PR is intended for merge then do not add such code at all.
BTW: please do not add @author
headers. We already have spent quite some effort to get rid of them. The latest oasp4j-ide config should not contain this in the templates anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed this file should have been deleted
@mathieu-lavigne when do you plan to take reviews in consideration? I think SPNG4 would need this piece of code... |
Hi @hohwille and @geoffroya I will work on this PR next week to make it ready for merge. Thanks for the comments. |
How can we proceed with this PR? Can we expect an update with merge errors resolved and travis working? Or should we find someone from the core team to take over. |
|
No description provided.